Track boundary interaction #2736

Sumit112192 · 2024-07-24T10:39:08Z

📝 Description

Type: 🚀 feature
This PR implements the functionality of tracking boundary interactions. It currently aims to track boundary interaction info in the form of (current_shell_id, next_shell_id)

Depends on PR #2719

codecov · 2024-07-24T11:03:28Z

Codecov Report

Attention: Patch coverage is 28.57143% with 30 lines in your changes missing coverage. Please review.

Project coverage is 69.38%. Comparing base (e1aa887) to head (1ea86d4).

Files	Patch %	Lines
tardis/transport/montecarlo/packet_trackers.py	13.33%	26 Missing ⚠️
tardis/transport/montecarlo/single_packet_loop.py	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2736      +/-   ##
==========================================
- Coverage   69.98%   69.38%   -0.61%     
==========================================
  Files         196      196              
  Lines       15008    15033      +25     
==========================================
- Hits        10503    10430      -73     
- Misses       4505     4603      +98

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tardis-bot · 2024-07-28T18:02:17Z

*beep* *bop*
Hi human,
I ran ruff on the latest commit (699cd4e).
Here are the outputs produced.
Results can also be downloaded as artifacts here.
Summarised output:

3	I001  	[*] Import block is un-sorted or un-formatted
2	PIE790	[*] Unnecessary `pass` statement
2	D411  	[*] Missing blank line before section ("Parameters")
2	UP004 	[*] Class `RPacketLastInteractionTracker` inherits from `object`
1	ANN205	[ ] Missing return type annotation for staticmethod `get_relative_path`
1	B009  	[*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
1	RET506	[ ] Unnecessary `else` after `raise` statement

Complete output(might be large):

benchmarks/benchmark_base.py:29:9: ANN205 Missing return type annotation for staticmethod `get_relative_path`
benchmarks/benchmark_base.py:75:9: RET506 Unnecessary `else` after `raise` statement
tardis/transport/montecarlo/packet_trackers.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/transport/montecarlo/packet_trackers.py:36:22: UP004 [*] Class `RPacketTracker` inherits from `object`
tardis/transport/montecarlo/packet_trackers.py:39:5: D411 [*] Missing blank line before section ("Parameters")
tardis/transport/montecarlo/packet_trackers.py:203:48: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
tardis/transport/montecarlo/packet_trackers.py:223:37: UP004 [*] Class `RPacketLastInteractionTracker` inherits from `object`
tardis/transport/montecarlo/packet_trackers.py:226:5: D411 [*] Missing blank line before section ("Parameters")
tardis/transport/montecarlo/packet_trackers.py:268:9: PIE790 [*] Unnecessary `pass` statement
tardis/transport/montecarlo/packet_trackers.py:275:9: PIE790 [*] Unnecessary `pass` statement
tardis/transport/montecarlo/single_packet_loop.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/transport/montecarlo/tests/test_rpacket_tracker.py:1:1: I001 [*] Import block is un-sorted or un-formatted
Found 12 errors.
[*] 10 fixable with the `--fix` option.

Sumit112192 · 2024-07-29T11:44:42Z

The output currently looks like:

rpacket_tracker = sim.transport.transport_state.rpacket_tracker

rpacket_tracker[2].boundary_interaction

array([(1, -1,  0), (2,  0,  1), (3,  1,  0), (4,  0, -1)],
      dtype=[('interaction_id', '<i8'), ('current_shell_id', '<i8'), ('next_shell_id', '<i8')])

rpacket_tracker[2].boundary_interaction["interaction_id"]

array([1, 2, 3, 4])

rpacket_tracker[2].boundary_interaction["current_shell_id"]

array([-1,  0,  1,  0])

rpacket_tracker[2].boundary_interaction["next_shell_id"]

array([ 0,  1,  0, -1])

wkerzendorf · 2024-07-29T17:13:34Z

tardis/transport/montecarlo/single_packet_loop.py

@@ -85,6 +86,9 @@ def single_packet_loop(
    )

    rpacket_tracker.track(r_packet)
+    rpacket_tracker.track_boundary_interaction(
+        PacketEjectaStatus.OUT_EJECTA, r_packet.current_shell_id


I think we should rename this @andrewfullard

I would personally just add to the PacketStatus Enum rather than defining a new one

Sumit112192 · 2024-07-30T09:59:02Z

@wkerzendorf I have added track line interaction in this PR, to demonstrate how will the final tracker functionality might look like instead of creating a new PR.

Sumit112192 · 2024-07-30T12:51:53Z

@wkerzendorf The output the tracker displays.

rpacket_tracker[10].boundary_interaction

array([( 1, -1,  0), ( 2,  0,  1), ( 3,  1,  2), ( 4,  2,  3),
       ( 5,  3,  4), ( 6,  4,  5), ( 7,  5,  4), ( 8,  4,  3),
       ( 9,  3,  2), (11,  2,  3), (12,  3,  4), (13,  4,  5),
       (14,  5,  6), (15,  6,  7), (16,  7,  8), (17,  8,  9),
       (20,  9, 10), (23, 10, 11), (25, 11, 10), (26, 10,  9),
       (27,  9,  8), (28,  8,  7), (29,  7,  6), (30,  6,  5),
       (31,  5,  4), (32,  4,  3), (34,  3,  4), (35,  4,  5),
       (37,  5,  6), (38,  6,  7), (39,  7,  8), (40,  8,  9),
       (41,  9, 10), (42, 10, 11), (43, 11, 12), (44, 12, 13),
       (45, 13, 14), (46, 14, 15), (47, 15, 16), (48, 16, 17),
       (49, 17, 18), (50, 18, 19), (51, 19, -1)],
      dtype=[('interaction_id', '<i8'), ('current_shell_id', '<i8'), ('next_shell_id', '<i8')])

rpacket_tracker[10].line_interaction

array([(10,  2, 1.38711254e+15, 1.96697997e+15, 4424, 4424),
       (18,  9, 1.70933416e+15, 2.09341849e+15, 4456, 3515),
       (19,  9, 1.72672264e+15, 2.75476977e+15, 3517, 3276),
       (21, 10, 1.75272138e+15, 3.04074219e+15, 3283, 5399),
       (22, 10, 1.76448678e+15, 1.68038515e+15, 5406, 5406),
       (24, 11, 1.79317922e+15, 1.66948517e+15, 5431, 5431),
       (33,  3, 1.41930616e+15, 1.61094915e+15, 5501, 4273),
       (36,  5, 1.51352255e+15, 2.16048881e+15, 4285, 7272)],
      dtype=[('interaction_id', '<i8'), ('shell_id', '<i8'), ('r', '<f8'), ('in_nu', '<f8'), ('in_id', '<i8'), ('out_id', '<i8')])

We can also access the individual attributes,

rpacket_tracker[10].line_interaction["interaction_id"]

array([10, 18, 19, 21, 22, 24, 33, 36])

rpacket_tracker[10].line_interaction["shell_id"]

array([ 2,  9,  9, 10, 10, 11,  3,  5])

rpacket_tracker[10].line_interaction["r"]

array([1.38711254e+15, 1.70933416e+15, 1.72672264e+15, 1.75272138e+15,
       1.76448678e+15, 1.79317922e+15, 1.41930616e+15, 1.51352255e+15])

rpacket_tracker[10].line_interaction["in_nu"]

array([1.96697997e+15, 2.09341849e+15, 2.75476977e+15, 3.04074219e+15,
       1.68038515e+15, 1.66948517e+15, 1.61094915e+15, 2.16048881e+15])

Sumit112192 · 2024-07-30T13:26:45Z

I have converted this PR to a draft. I have to add tests, but it still can be reviewed as tests are independent.

andrewfullard · 2024-07-30T13:27:36Z

tardis/transport/montecarlo/packet_trackers.py

+        self.event_id += 1
+
+    def track_line_interaction(self, r_packet):
+        if self.num_line_interactions >= self.line_interaction_array_length:


I think this if block, and the one in track_boundary_interaction, may be better as a separate method that performs the array extension.

Yes. That makes sense. I will look into it.

Can they both be merged into a single extend function?

That would be ideal.

Sumit112192 · 2024-07-31T17:53:38Z

tardis/transport/montecarlo/packet_trackers.py

        self.num_interactions = 0
+        self.boundary_interactions_index = 0


@wkerzendorf If I keep the name num_boundary_interactions it seems similar to the variable num_interactions and can be used to obtain the number of boundary interactions whereas with the current name, it seems that it will only be used for array purposes. Should I rename it to num_boundary_interactions

It will be used for indexing purposes - essentially all of the analysis will be done on the dataframes

Sumit112192 · 2024-07-31T17:56:46Z

tardis/transport/montecarlo/packet_trackers.py

    """

    def __init__(self, length):
-        self.length = length


I removed this length variable, since If I kept this length, I would have to add another length variable for every track_interaction function.

Sumit112192 · 2024-08-02T08:05:38Z

why is black failing here? even though locally with the same version it says no reformat is required. @atharva-2001

atharva-2001 · 2024-08-02T08:10:20Z

Are you sure your black version is the same? black==22.3? What happens when you do black --version
Are you running the same command locally as in the workflow? What command are you running?

Sumit112192 · 2024-08-02T08:12:01Z

Are you sure your black version is the same? black==22.3? What happens when you do black --version

Are you running the same command locally as in the workflow? What command are you running?

Yeah. I checked the version and the file it says to reformat, I ran black on that file.

Sumit112192 · 2024-08-02T08:14:02Z

I haven't even changed that file, black is suggesting.

atharva-2001 · 2024-08-02T08:16:27Z

Can you rebase once?

Sumit112192 · 2024-08-02T08:17:29Z

Can you rebase once?

I did that just now.

atharva-2001 · 2024-08-02T08:30:46Z

I will try replicating this in a while

Sumit112192 · 2024-08-02T08:32:49Z

I will try replicating this in a while

thanks.

Sumit112192 · 2024-08-05T08:50:40Z

To replicate the benchmarking error,I started with a new branch created from the upstream/master branch, I rerun the benchmark locally of every commit, I made. First, remove the length variable, then declare the boundary_interaction datatype, and then define the track_boundary_interaction function. The benchmark failed when I added the track_boundary_interaction so the issue is this function but I can't figure it out. @officialasishkumar @andrewfullard Can you help me figure out the problem with this function?

… as RPacketTracker

Sumit112192 force-pushed the TrackBoundaryInteraction branch 2 times, most recently from 771a665 to 7506cac Compare July 29, 2024 05:31

Sumit112192 marked this pull request as ready for review July 29, 2024 13:49

wkerzendorf reviewed Jul 29, 2024

View reviewed changes

Sumit112192 force-pushed the TrackBoundaryInteraction branch from f5242e4 to edd7f43 Compare July 30, 2024 08:44

Sumit112192 marked this pull request as draft July 30, 2024 13:25

andrewfullard reviewed Jul 30, 2024

View reviewed changes

Sumit112192 force-pushed the TrackBoundaryInteraction branch from 1a6c053 to 6811d77 Compare July 31, 2024 16:11

Sumit112192 commented Jul 31, 2024

View reviewed changes

Sumit112192 force-pushed the TrackBoundaryInteraction branch from 6524edd to 909b73c Compare August 2, 2024 08:11

Sumit112192 marked this pull request as ready for review August 2, 2024 13:35

andrewfullard previously approved these changes Aug 2, 2024

View reviewed changes

andrewfullard requested a review from wkerzendorf August 2, 2024 13:48

Sumit112192 force-pushed the TrackBoundaryInteraction branch from 0ff14f5 to 8987037 Compare August 5, 2024 04:50

Sumit112192 added 23 commits August 10, 2024 23:03

Initializae self.line_interaction_array_length

ae6dc64

Fix Typo

593297e

Rename interaction_id to event_id

04cf212

Add doc strings to the functions

be2326d

Add more doc strings

4289974

Remove Line Interaction from this PR

675deb7

Remove Track Line Interaction From this PR

ebba27a

Use Extend array function

9883bc7

Fix Typo

7d799dc

Remove length attribute from the class as it was clustering the code

1024f57

Remove line interaction from this PR

3c2d733

Add tests

d31b199

Remove print statement

939fbbe

Update test

f4ac83e

Trigger Build

c1a0b92

Rename track_boundary_interaction to boundary_interactions_track

8eafee1

Rename from boundary_interactions_track to get_boundary_data

e657f95

Rename from get_boundary_data to get_boundary_interaction

42c460d

Rename function name in RPacketLastInteractionTracker to make it same…

2ce9cb9

… as RPacketTracker

Change the way RPacketTracker is imported

826a2d9

Use RPacketLastInteractionTracker since it is set by default

3d2ff15

Remove ENUM

2b5c1f8

Max column 60

699cd4e

Sumit112192 force-pushed the TrackBoundaryInteraction branch from 7ee0314 to 699cd4e Compare August 10, 2024 17:33

andrewfullard added the restructure label Aug 12, 2024

andrewfullard enabled auto-merge (squash) August 12, 2024 14:11

andrewfullard approved these changes Aug 12, 2024

View reviewed changes

Rodot- approved these changes Aug 12, 2024

View reviewed changes

andrewfullard merged commit 63eb762 into tardis-sn:master Aug 12, 2024
23 of 25 checks passed

Sumit112192 deleted the TrackBoundaryInteraction branch September 8, 2024 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track boundary interaction #2736

Track boundary interaction #2736

Sumit112192 commented Jul 24, 2024

codecov bot commented Jul 24, 2024 •

edited

Loading

tardis-bot commented Jul 28, 2024 •

edited

Loading

Sumit112192 commented Jul 29, 2024

wkerzendorf Jul 29, 2024

andrewfullard Jul 29, 2024

Sumit112192 commented Jul 30, 2024 •

edited

Loading

Sumit112192 commented Jul 30, 2024

Sumit112192 commented Jul 30, 2024

andrewfullard Jul 30, 2024

Sumit112192 Jul 30, 2024

Sumit112192 Jul 30, 2024

andrewfullard Jul 30, 2024

Sumit112192 Jul 31, 2024

wkerzendorf Aug 1, 2024

Sumit112192 Jul 31, 2024

Sumit112192 commented Aug 2, 2024

atharva-2001 commented Aug 2, 2024 •

edited

Loading

Sumit112192 commented Aug 2, 2024

Sumit112192 commented Aug 2, 2024

atharva-2001 commented Aug 2, 2024

Sumit112192 commented Aug 2, 2024

atharva-2001 commented Aug 2, 2024

Sumit112192 commented Aug 2, 2024

Sumit112192 commented Aug 5, 2024 •

edited

Loading

		self.num_interactions = 0
		self.boundary_interactions_index = 0

Track boundary interaction #2736

Track boundary interaction #2736

Conversation

Sumit112192 commented Jul 24, 2024

📝 Description

codecov bot commented Jul 24, 2024 • edited Loading

Codecov Report

tardis-bot commented Jul 28, 2024 • edited Loading

Sumit112192 commented Jul 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sumit112192 commented Jul 30, 2024 • edited Loading

Sumit112192 commented Jul 30, 2024

Sumit112192 commented Jul 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sumit112192 commented Aug 2, 2024

atharva-2001 commented Aug 2, 2024 • edited Loading

Sumit112192 commented Aug 2, 2024

Sumit112192 commented Aug 2, 2024

atharva-2001 commented Aug 2, 2024

Sumit112192 commented Aug 2, 2024

atharva-2001 commented Aug 2, 2024

Sumit112192 commented Aug 2, 2024

Sumit112192 commented Aug 5, 2024 • edited Loading

codecov bot commented Jul 24, 2024 •

edited

Loading

tardis-bot commented Jul 28, 2024 •

edited

Loading

Sumit112192 commented Jul 30, 2024 •

edited

Loading

atharva-2001 commented Aug 2, 2024 •

edited

Loading

Sumit112192 commented Aug 5, 2024 •

edited

Loading